Conversation
max-nextcloud
force-pushed
the
fix/474-unique-id-for-content
branch
from
ee53f8e to
67934c8
Compare
`content` is already used as an id in the server template: https://github.com/nextcloud/server/blob/c10317f7f9f53a4de464915f754896eed7f1ee6c/core/templates/layout.user.php#L125 Make sure to have a unique id and to replace the correct dom element when mounting the vue app. The app uses NcAppContent which is rendered as a `<main>` tag. Mount the app on `main#content` from the server template so there is only one `<main>` on the page. Signed-off-by: Max <max@nextcloud.com>
max-nextcloud
force-pushed
the
fix/474-unique-id-for-content
branch
from
67934c8 to
76e3676
Compare
|
I replaced the |
| @@ -1 +1,8 @@ | |||
| <div id="content"></div> | |||
There was a problem hiding this comment.
well this element is replaced anyway on mounting
I had a failure in my comment. Why actually should be done is:
https://github.com/nextcloud/app-tutorial/blob/master/src/App.vue#L2
Should use NcContent
https://nextcloud-vue-components.netlify.app/#/Components/App%20containers?id=nccontent
There was a problem hiding this comment.
I agree about using NcContent - I thought I'd do that in another PR.
But I think the element that is replaced on mounting is actually this:
https://github.com/nextcloud/server/blob/master/core/templates/layout.user.php#L125
It's a parent element so the div#content will also be removed.
I'm not sure what vue will do if the id is used twice. What i am trying to do here is to avoid that ambiguity.
There was a problem hiding this comment.
But I think the element that is replaced on mounting is actually this:
https://github.com/nextcloud/server/blob/master/core/templates/layout.user.php#L125
Which is exactly why your PR is "wrong". That element must not be removed/replaced.
There was a problem hiding this comment.
Uh wow... but then we cannot use NcAppContent anymore because
A document must not have more than one main element that does not have the hidden attribute specified.
https://html.spec.whatwg.org/multipage/grouping-content.html#the-main-element
Or rather we should turn that main element into a div and use a different id then content on it. Which would basically revert nextcloud/server@a8ff2ff#diff-190a82b500ef1394d8a0cb94acb6225c1416a9d7d6340f08432da49fc5c98eba . But that was introduced on purpose as well. So looks like there's no easy solution here.
There was a problem hiding this comment.
That element must not be removed/replaced.
But that's already happening anyway.
Here's the dom in current master branch before the vue mount:
And this is the dom afterwards:
There was a problem hiding this comment.
Hmm right, seems like talk also as an empty template:
https://github.com/nextcloud/spreed/blob/master/templates/index.php
I thought we always nest. But then it seems your current patch is fine.
|
Closing as stale ;) |
3 participants
contentis already used as an id in the server template: https://github.com/nextcloud/server/blob/c10317f7f9f53a4de464915f754896eed7f1ee6c/core/templates/layout.user.php#L125Make sure to have a unique id and to replace the correct dom element when mounting the vue app.
Fixes #474.